fix(icons): reduce max_url_length to 3550 and add camo limit warnings - #109
Conversation
Co-authored-by: ChipWolf <3164166+ChipWolf@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR fixes GitHub camo proxy URL length limit issues by reducing the max_url_length parameter from 3700 to 3550 characters. The tighter limit ensures that badge URLs with embedded SVG data URIs stay under GitHub's camo proxy 8192 character limit, preventing "limit exceeded" errors for large icons like the Amazon AWS icon.
Key Changes
- Reduced default
max_url_lengthfrom 3700 to 3550 insvg_to_base64_data_uri()function - Updated all explicit call sites to use 3550
- Updated test assertions to reflect the new default value
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| badgesort/icons.py | Updated function signature default parameter, docstring calculation, and two explicit call sites to use 3550 char limit |
| tests/test_camo_url_limits.py | Updated test assertions and comments to verify the new 3550 char default value |
| Default 3550 chars ensures badge URLs stay under GitHub's camo proxy | ||
| 8192 char limit (which hex-encodes URLs: 76 + url_len*2 <= 8192). | ||
| Calculation: safe_url = (8192 - 76) / 2 - overhead ≈ 3850, with 150 char margin = 3700 | ||
| Calculation: safe_url = (8192 - 76) / 2 - overhead ≈ 3949, with 10% margin = 3550 |
There was a problem hiding this comment.
[nitpick] The calculation in the docstring could be clearer. Consider specifying what "overhead" refers to:
Calculation: safe_url = (8192 - 76) / 2 - ~109 badge_url_overhead ≈ 3949, with 10% margin = 3550or
Calculation: (8192 - 76 camo_overhead) / 2 hex_encoding - ~109 badge_url_overhead = 3949, with 10% margin = 3550This makes it clear that the ~109 char overhead is the badge URL structure (domain, path, parameters) excluding the actual data URI payload.
| Calculation: safe_url = (8192 - 76) / 2 - overhead ≈ 3949, with 10% margin = 3550 | |
| Calculation: (8192 - 76 camo_overhead) / 2 hex_encoding - ~109 badge_url_overhead = 3949, with 10% margin = 3550 | |
| (~109 char badge_url_overhead is the badge URL structure: domain, path, parameters, excluding the data URI payload) |
There was a problem hiding this comment.
Updated the docstring for better clarity in commit fb06706. The calculation now explicitly labels each overhead component and explains what badge_url_overhead represents.
|
The test failed in CI @copilot |
Co-authored-by: ChipWolf <3164166+ChipWolf@users.noreply.github.com>
... Fixed in commit fb06706. The CI failure was due to missing |
|
@copilot Use the BadgeSort container in CI instead of this |
Co-authored-by: ChipWolf <3164166+ChipWolf@users.noreply.github.com>
Updated in commit 0ba8bc1. CI now builds and uses the BadgeSort Docker container to run tests, which includes all system dependencies (librsvg2-bin) by default. Also added console warnings when badge URLs exceed the GitHub camo limit or when PNG fallback fails. |
| Default 3550 chars ensures badge URLs stay under GitHub's camo proxy | ||
| 8192 char limit (which hex-encodes URLs: 76 + url_len*2 <= 8192). | ||
| Calculation: safe_url = (8192 - 76) / 2 - overhead ≈ 3850, with 150 char margin = 3700 | ||
| Calculation: (8192 - 76 camo_overhead) / 2 hex_encoding - ~109 badge_url_overhead = 3949, with 10% margin = 3550 |
There was a problem hiding this comment.
[nitpick] The calculation description states "with 10% margin = 3550" but this is slightly unclear. The actual calculation would be: 3949 × 0.9 ≈ 3554, but the code uses 3550 (which is even more conservative). Consider clarifying the description to either:
- "with ~10% margin (rounded down) = 3550" or
- "with 399 char safety margin (≈10%) = 3550"
This would make it clearer that 3550 is a rounded-down value for safety.
| Calculation: (8192 - 76 camo_overhead) / 2 hex_encoding - ~109 badge_url_overhead = 3949, with 10% margin = 3550 | |
| Calculation: (8192 - 76 camo_overhead) / 2 hex_encoding - ~109 badge_url_overhead = 3949, with ~10% margin (rounded down) = 3550 |
Fix failing camo length test
This PR fixes GitHub camo proxy URL length limit issues by reducing the
max_url_lengthparameter from 3700 to 3550 characters and adding user-facing warnings when URLs exceed safe limits.Changes Made
max_url_length: Changed default from 3700 to 3550 chars insvg_to_base64_data_uri()to account for badge URL overhead and provide 10% safety marginTechnical Details
Calculation:
(8192 - 76 camo_overhead) / 2 hex_encoding - ~109 badge_url_overhead = 3949, with 10% margin = 3550The tighter limit ensures that badge URLs with embedded SVG data URIs stay under GitHub's camo proxy 8192 character limit, preventing "limit exceeded" errors for large icons like the Amazon AWS icon. When PNG fallback is available (via rsvg-convert), oversized SVGs are automatically converted to compact PNGs.
Testing
Fixes #108
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.